Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

VACMS-19516 Adjust email container structure and spacing #2348

Merged
merged 1 commit into from
Nov 7, 2024

Conversation

randimays
Copy link
Contributor

@randimays randimays commented Nov 7, 2024

Summary

Adjust the container structure of the email signup widget. The current version has a lot of nested divs that make it difficult to properly center the form with the proper length of the text input intact. The current/original version is unchanged, I simply isolated those extra divs to that version and simplified the implementation of the new React widget so I could style it properly.

Related issue(s)

Testing done

Since the React widget code has not been merged yet, it is more important to validate this style/structure change against the existing email form. But I did add screenshots of the React widget form because I have a branch of that code locally.

Screenshots

Current version (unchanged)

Screenshot 2024-11-07 at 1 47 37 PM Screenshot 2024-11-07 at 1 47 29 PM Screenshot 2024-11-07 at 1 47 21 PM

React widget

Screenshot 2024-11-07 at 1 42 15 PM Screenshot 2024-11-07 at 1 41 49 PM Screenshot 2024-11-07 at 1 41 16 PM

What areas of the site does it impact?

Homepage email signup widget only.

@@ -1,47 +1,47 @@
<div class="homepage-email-update-wrapper vads-u-background-color--primary-alt-lightest vads-u-padding-x--2p5 vads-u-padding-top--2p5">
{% assign shouldShowExistingEmailForm = '' | showExistingEmailForm %}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assign / if moved from lines 4-6 to lines 2-4. Then the else and endif moved from lines 40-42 to lines 42-44. The rest of the code is unchanged.

Copy link
Contributor

@chriskim2311 chriskim2311 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@randimays randimays merged commit e7b05fc into main Nov 7, 2024
27 checks passed
@randimays randimays deleted the 19516-adjust-email-spacing branch November 7, 2024 22:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants